Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a complete CLI for Jira (jcli): command handlers, config and state persistence, Jira HTTP client and types, TUI selector, branch-name generator, tests (unit and integration), CI/CD and release workflows, Makefile, goreleaser config, and documentation. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant CLI as CLI
participant Config as Config
participant Jira as JiraClient
participant TUI as TUI
participant State as State
User->>CLI: issue select
CLI->>Config: Load()
Config-->>CLI: Config (project/status)
CLI->>Jira: SearchIssues(project,status)
Jira-->>CLI: []Issue
CLI->>TUI: SelectIssue(issues)
TUI->>User: show options
User-->>TUI: choose
TUI-->>CLI: Selected Issue
CLI->>State: SetCurrentIssue(key,summary)
State->>State: write state.json
State-->>CLI: success
CLI-->>User: confirm selection
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yaml:
- Around line 85-86: The integration test step ("Run integration tests") runs
unconditionally and will fail if required Jira secrets are missing; update the
CI job to gate or skip this step when secrets are absent by adding a conditional
check and/or a pre-step that sets a flag. Concretely, detect the required
secrets (e.g., secrets.JIRA_USERNAME and secrets.JIRA_TOKEN) before running the
command referenced in the step (the go test invocation "go test -v
-tags=integration ./tests/integration/...") and either (a) add an if:
conditional on the step to only run when those secrets are present, or (b) add a
short preparatory step that exits/sets SKIP_INTEGRATION when secrets are missing
and then guard the "Run integration tests" step on that flag so tests are
skipped rather than failing.
- Around line 15-18: Update the GitHub Actions steps that use
actions/setup-go@v5 to read the Go version from go.mod instead of hardcoding
go-version; specifically, replace the go-version: '1.22' (and the other
hardcoded go-version entries) with go-version-file: 'go.mod' in each setup step
that references actions/setup-go@v5 so the CI toolchain matches the go.mod
declared version.
In @.github/workflows/release.yaml:
- Around line 26-28: Guard the "Run integration tests" step so it only runs when
Jira credentials are available and pass those secrets into the job; update the
step named "Run integration tests" to include an if condition checking e.g. ${{
secrets.JIRA_USERNAME && secrets.JIRA_API_TOKEN }} (or the specific secret names
your repo uses) and add the required secrets as environment variables (e.g. env:
JIRA_USERNAME: ${{ secrets.JIRA_USERNAME }}, JIRA_API_TOKEN: ${{
secrets.JIRA_API_TOKEN }}) so the go test command (go test -v -tags=integration
./tests/integration/...) has access to them.
In @.goreleaser.yaml:
- Around line 44-50: The release configuration currently sets
release.github.owner to "dk" which is incorrect for this repository; update the
owner field in .goreleaser.yaml from "dk" to "tutunak" so the release target
matches the repository (ensure the YAML key is release -> github -> owner and
the value is changed to tutunak).
In `@cmd/issue_branch.go`:
- Around line 16-20: The HasCurrentIssue() check in cmd/issue_branch.go prints
guidance but then returns fmt.Errorf("no issue selected"), which is inconsistent
with executeIssueCurrent (which returns nil); change the return to nil for
consistency: in the function containing the HasCurrentIssue() block replace the
fmt.Errorf("no issue selected") return with return nil so the guidance is
printed but no error is propagated (or alternatively, if you prefer error
semantics, update executeIssueCurrent to return an error too—pick one approach
and apply it consistently across the HasCurrentIssue() checks and their
callers).
In `@go.mod`:
- Line 1: Update the module path in go.mod from module github.com/dk/jcli to
module github.com/tutunak/jcli and then update any installation/import docs
(e.g., README instructions that show go install or import paths) to use
github.com/tutunak/jcli so builds and go install resolve to the canonical
repository; ensure the module line in go.mod is the only change required in
source files (update any hard-coded import paths if present).
In `@internal/config/config.go`:
- Around line 62-65: When os.ReadFile(path) returns a not-found error the
function currently returns immediately so environment variable overrides are
never applied; change the early-return branch to call applyEnvOverrides(&cfg)
(the function that applies env vars) before returning so CLI config can be
driven purely by environment variables, and ensure other error paths still
return the read error (i.e., only treat os.IsNotExist(err) as "no file" but
still apply applyEnvOverrides on cfg).
In `@internal/jira/client_test.go`:
- Around line 59-94: The test HTTP handler uses json.NewEncoder(w).Encode(issue)
without checking its error, which fails errcheck in CI; update the handler
inside TestHTTPClient_GetIssue (the httptest.NewServer http.HandlerFunc) to
capture the error returned by json.NewEncoder(w).Encode(issue) and handle it by
calling t.Fatalf or t.Errorf with the error (or at least t.Fatal on encode
failure) so the test fails clearly; apply the same fix to other test handlers
with json.NewEncoder(...).Encode(...) (e.g., similar handlers earlier in the
file) to satisfy errcheck.
- Around line 10-57: The test TestHTTPClient_SearchIssues calls
json.NewEncoder(w).Encode(result) but ignores the returned error; update the
handler in TestHTTPClient_SearchIssues to capture the error from
json.NewEncoder(w).Encode(result) and handle it (e.g., call t.Fatalf or t.Error
with the error) so the encode error is handled and satisfies errcheck/CI; ensure
the error handling occurs before returning from the handler to avoid silent
failures.
In `@Makefile`:
- Around line 11-15: The build-all Makefile target fails when the dist directory
is missing; update the build process for the build-all target to ensure dist
exists before running go build (e.g., add a dist prerequisite target or a mkdir
-p dist command at the start of the build-all recipe) so the four GOOS/GOARCH go
build -o dist/... invocations succeed; reference the build-all target and the
dist directory in your change.
- Around line 30-31: The install target currently copies the jcli binary into
$(GOPATH)/bin which becomes /bin when GOPATH is unset; change the install recipe
to resolve the install directory using `go env GOBIN` (fallback to `$(shell go
env GOPATH)/bin` if GOBIN is empty), ensure that directory exists (mkdir -p) and
then copy jcli into that resolved directory; update the install target (symbol:
install) to compute the destination via a shell call and create it before
executing cp.
In `@README.md`:
- Around line 113-118: The fenced code blocks following the "Output:" markers
are missing language identifiers; update each triple-backtick fenced block (the
blocks that contain the sample outputs such as the block with "Current issue:
PROJ-123..." and the block with "PROJ-123-implement-user-authentication-847291")
to include a language specifier (use "text") immediately after the opening ```
so they become ```text; ensure you apply the same change to the other affected
output blocks referenced in the comment.
- Around line 152-175: The README tables under the "Root Commands", "Issue
Commands", and "Config Commands" sections have inconsistent spacing around the
pipe characters; normalize all table rows to align pipe spacing uniformly (e.g.,
single space on both sides of cell content) so each header and row uses the same
pattern for `| Command | Description |` and similar rows (also apply the same
normalization to the other tables referenced at lines 193-198); update every row
and separator line for those tables to match the consistent pipe/space style to
satisfy MD060.
- Around line 59-64: The bare URL in the "Getting a Jira API Token" section must
be wrapped to satisfy MD034; replace the plain
https://id.atlassian.com/manage-profile/security/api-tokens with either a
markdown link (e.g., [Atlassian API
tokens](https://id.atlassian.com/manage-profile/security/api-tokens)) or an
angle-bracketed URL
(<https://id.atlassian.com/manage-profile/security/api-tokens>) inside the
"Getting a Jira API Token" list so the linter no longer flags the bare URL.
In `@tests/integration/integration_test.go`:
- Around line 170-213: Tests call the non-existent "task" root command; change
each runCLI invocation from "task" to "issue" and update the corresponding
assertion strings to match the actual CLI outputs produced by the issue command.
Locate the four t.Run cases named "task select by ID", "task current", "task
branch", and "task help" and in each replace runCLI("task", ...) with
runCLI("issue", ...), then revise the expected substrings used in
strings.Contains/HasPrefix checks to the actual messages returned by the issue
command (adjust the "Selected...", "Current...", branch prefix, and help text
assertions accordingly).
🧹 Nitpick comments (6)
internal/jira/mock.go (1)
16-19: Map should reference the slice element, not a copy.
Storing&issuepoints at a separate copy; future mutations viaGetIssuewon’t reflect inIssues. Use the slice element address to keep map/slice consistent.♻️ Proposed fix
func (m *MockClient) AddIssue(issue Issue) { m.Issues = append(m.Issues, issue) - m.IssueByKey[issue.Key] = &issue + m.IssueByKey[issue.Key] = &m.Issues[len(m.Issues)-1] }internal/config/config_test.go (1)
79-118: Consider adding a test case for environment overrides without a config file.The current test creates a config file before setting environment variables. Given the bug in
Load()whereapplyEnvOverrides()isn't called when the config file is missing, consider adding a test case that verifies environment variables work even when no config file exists.💡 Additional test case suggestion
func TestEnvOverridesWithoutConfigFile(t *testing.T) { tmpDir := t.TempDir() t.Setenv("XDG_CONFIG_HOME", tmpDir) // No config file created t.Setenv("JIRA_URL", "https://env.atlassian.net") t.Setenv("JIRA_EMAIL", "env@example.com") t.Setenv("JIRA_API_TOKEN", "env-token") loaded, err := Load() if err != nil { t.Fatalf("unexpected error: %v", err) } if loaded.Jira.URL != "https://env.atlassian.net" { t.Errorf("expected env URL override, got %q", loaded.Jira.URL) } // ... additional assertions }internal/branch/generator.go (1)
103-126: Consider usingstrconv.Itoainstead of custom implementation.The custom
itoaand related number formatting functions could be simplified by using the standard library'sstrconv.Itoa. This would improve readability and leverage well-tested code.♻️ Simplified implementation using strconv
+import ( + "strconv" + // ... other imports +) -func formatNumber(n int) string { - return strings.TrimPrefix(strings.TrimPrefix( - strings.TrimPrefix(strings.TrimPrefix( - strings.TrimPrefix(padNumber(n), "0"), "0"), "0"), "0"), "0") -} - -func padNumber(n int) string { - s := "000000" - num := n % 1000000 - result := s + itoa(num) - return result[len(result)-6:] -} - -func itoa(n int) string { - if n == 0 { - return "0" - } - var digits []byte - for n > 0 { - digits = append([]byte{byte('0' + n%10)}, digits...) - n /= 10 - } - return string(digits) -} +func formatNumber(n int) string { + return strconv.Itoa(n % 1000000) +}internal/branch/generator_test.go (1)
42-47: Consider handling empty summary case.The empty summary test shows that an empty summary results in a double hyphen in the branch name (
PROJ-999--847291). While this is functionally correct based on the current implementation, you may want to handle this edge case explicitly inGenerateto produce cleaner output likePROJ-999-847291.cmd/config.go (1)
120-129: Handle ConfigPath errors before printing.This avoids misleading output if the config path cannot be resolved.
♻️ Suggested fix
- configPath, _ := config.ConfigPath() + configPath, err := config.ConfigPath() + if err != nil { + return fmt.Errorf("failed to determine config path: %w", err) + }internal/tui/selector.go (1)
24-29: Avoid byte-slicing UTF‑8 labels.Byte slicing can split multi‑byte characters and render invalid text in the TUI. Consider truncating by runes instead.
♻️ Suggested fix
- if len(label) > 80 { - label = label[:77] + "..." - } + if len(label) > 80 { + r := []rune(label) + if len(r) > 80 { + label = string(r[:77]) + "..." + } + }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yaml:
- Around line 42-46: The CI step named "golangci-lint" currently uses `version:
latest`; replace that with a pinned tag (e.g., `v1.61.0` or your chosen tested
release) to ensure reproducible builds — update the `golangci-lint` action's
`version` input to a concrete version string and commit the change so the
workflow no longer tracks `latest`.
Comment on lines
+42
to
+46
| - name: golangci-lint | ||
| uses: golangci/golangci-lint-action@v6 | ||
| with: | ||
| version: latest | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find any existing golangci-lint version pins or references.
fd -a "\.golangci.*"
echo "---"
rg -n "golangci-lint|GOLANGCI_LINT_VERSION|GOLANGCI_LINT" -SRepository: tutunak/jcli
Length of output: 149
🏁 Script executed:
#!/bin/bash
# Check for .golangci config files including those that might be in .gitignore
git ls-files -o -i --exclude-standard | grep -i golangci
echo "---"
# Check all files including potentially ignored ones
find . -name ".golangci*" -o -name "golangci-lint*" 2>/dev/null | head -20
echo "---"
# Check for version constraints in go.mod
if [ -f go.mod ]; then
rg "golangci-lint" go.mod
fi
echo "---"
# Read the ci.yaml file to see full context
cat -n .github/workflows/ci.yaml | head -50Repository: tutunak/jcli
Length of output: 1355
Pin golangci-lint to a specific version instead of latest.
Using version: latest undermines CI reproducibility; other action versions (checkout, setup-go) and the Go version itself are pinned, but golangci-lint is left unpinned and can introduce breaking changes without notice. Specify a concrete version (e.g., v1.61.0 or newer) aligned with your project's requirements.
♻️ Suggested change
- name: golangci-lint
uses: golangci/golangci-lint-action@v6
with:
- version: latest
+ version: v1.61.0📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: golangci-lint | |
| uses: golangci/golangci-lint-action@v6 | |
| with: | |
| version: latest | |
| - name: golangci-lint | |
| uses: golangci/golangci-lint-action@v6 | |
| with: | |
| version: v1.61.0 | |
🤖 Prompt for AI Agents
In @.github/workflows/ci.yaml around lines 42 - 46, The CI step named
"golangci-lint" currently uses `version: latest`; replace that with a pinned tag
(e.g., `v1.61.0` or your chosen tested release) to ensure reproducible builds —
update the `golangci-lint` action's `version` input to a concrete version string
and commit the change so the workflow no longer tracks `latest`.
There was a problem hiding this comment.
Pull request overview
Introduces a new jcli Jira-focused CLI with config/state persistence, interactive selection UI, branch name generation, and accompanying CI/release automation.
Changes:
- Added CLI entrypoint and command routing (
issue,config,version/help) plus supporting packages (Jira client, config, state, TUI selector, branch generator). - Added unit and integration tests for key behaviors.
- Added project docs and automation (Makefile, GitHub Actions CI/release, GoReleaser config, repo hygiene files).
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/integration_test.go | Integration test that builds and exercises the CLI end-to-end. |
| main.go | CLI entrypoint wiring version injection and command execution. |
| internal/tui/selector_test.go | Unit tests for selector construction and empty-list behavior. |
| internal/tui/selector.go | TUI issue selector and interactive credentials prompt. |
| internal/state/state_test.go | Unit tests for XDG state paths and persisted current-issue behavior. |
| internal/state/state.go | JSON-backed state persistence for current issue selection. |
| internal/jira/types.go | Jira API types used by the client and UI. |
| internal/jira/mock.go | Mock Jira client for unit testing. |
| internal/jira/client_test.go | Unit tests for Jira HTTP client and mock client behavior. |
| internal/jira/client.go | Jira REST API client implementation (search + get issue). |
| internal/config/config_test.go | Unit tests for config defaults, persistence, and env overrides. |
| internal/config/config.go | YAML-backed config persistence + env var overrides + validation. |
| internal/branch/generator_test.go | Unit tests for branch name generation and normalization. |
| internal/branch/generator.go | Branch name generation and summary normalization logic. |
| go.sum | Module dependency lockfile. |
| go.mod | Module definition and dependency requirements. |
| cmd/root.go | Top-level CLI argument parsing and routing. |
| cmd/issue_select.go | issue select implementation (by key and interactive). |
| cmd/issue_current.go | issue current implementation. |
| cmd/issue_branch.go | issue branch implementation. |
| cmd/issue.go | issue subcommand router + usage output. |
| cmd/config.go | config subcommand handlers (project/status/credentials/show). |
| README.md | User-facing usage and setup documentation. |
| Makefile | Build/test/lint targets and multi-arch build outputs. |
| CLAUDE.md | Developer guidance for working in this repo. |
| .goreleaser.yaml | GoReleaser configuration for packaging/releases. |
| .gitignore | Ignore rules for binaries, build artifacts, and tooling output. |
| .github/workflows/release.yaml | Release workflow including tests and GoReleaser run. |
| .github/workflows/ci.yaml | CI workflow for test/lint/build/integration checks. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+54
to
+75
| func Load() (*Config, error) { | ||
| path, err := ConfigPath() | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| cfg := DefaultConfig() | ||
|
|
||
| data, err := os.ReadFile(path) | ||
| if os.IsNotExist(err) { | ||
| return cfg, nil | ||
| } | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to read config file: %w", err) | ||
| } | ||
|
|
||
| if err := yaml.Unmarshal(data, cfg); err != nil { | ||
| return nil, fmt.Errorf("failed to parse config file: %w", err) | ||
| } | ||
|
|
||
| cfg.applyEnvOverrides() | ||
| return cfg, nil |
There was a problem hiding this comment.
Load() returns early when the config file doesn’t exist, so environment variable overrides (e.g., JIRA_API_TOKEN, JIRA_URL, etc.) are not applied unless a config file is present. Apply cfg.applyEnvOverrides() before returning in the os.IsNotExist(err) branch so env-only configuration works.
Owner
Author
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: tutunak <12380949+tutunak@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.